-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
native: add option to turn RIOT into a Linux CLI tool #20656
base: master
Are you sure you want to change the base?
Conversation
Cool idea! How about making it behave like busybox? I dislike that there is a new example that does not really have value as it - add the very least add a README! I am also surprised it is nanocoap specific? While coap is a good example use case, the example itself is more "riot as cli tool" and should be presented as such. |
I agree that the example needs to be more fleshed out, this is a bit specific to the 'send arbitrary CoAP payloads as a RIOT node would' use case, but not sure what else I should include. |
Is this ready for a review @benpicco ? |
Sure! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall. Only minor comments and nits.
examples/nanocoap_tool/Makefile
Outdated
USEMODULE += shell | ||
USEMODULE += shell_cmds_default | ||
|
||
# export the whole fs to RIOT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? Wouldn't be "./" be sufficient for an example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Paths in RIOT are always absolute, we don't have the concept of a current directory.
So to access main.c
in the directory of the example we'd have to write /main.c
which does not match the host file system.
examples/nanocoap_tool/main.c
Outdated
} | ||
|
||
/* wait some time for the network to be ready */ | ||
ztimer_sleep(ZTIMER_MSEC, 1000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥲
sys/shell/cmds/nanocoap_vfs.c
Outdated
@@ -169,19 +169,34 @@ static int _nanocoap_put_handler(int argc, char **argv) | |||
file = argv[1]; | |||
url = argv[2]; | |||
|
|||
/* append filename to remote dir */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? (I also find the difference between the comments wording "filename" & "remote dir" confusing, given that the variables are called "file" and "url", but that's very much nit picking.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was there before, so when you write
ncput /path/to/foo.txt coap://example.com/in/
it will actually make a PUT request to coap://example.com/in/foo.txt
Does changing it to 'remote URL' make it clearer?
sys/shell/cmds/nanocoap_vfs.c
Outdated
puts("Constructed URI too long"); | ||
return -ENOBUFS; | ||
} | ||
url = buffer; | ||
} | ||
|
||
/* relative file path */ | ||
if (*file != '/') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that would make it more clear that inspecting the first character only is deliberate. (Again, nit)
if (*file != '/') { | |
if (file[0] != '/') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, this if is also take if file == "-"
. I think that is okay and can be ignored?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was added as a convenience. We already have CONFIG_NCGET_DEFAULT_DATA_DIR
to store downloads from ncget
when the destination is not specified - so you can do
ncget coap://example.com/files/data.txt
and it will end up in CONFIG_NCGET_DEFAULT_DATA_DIR
.
This is just the reverse: It allows you do to
ncput foo.txt coap://example.com/in/
without typing out the whole path.
A README for the example would be nice. It took me a while to wrap my head around the concept. |
I added a readme and some wrapper scripts |
9894968
to
7701fc5
Compare
7701fc5
to
d3dbcbc
Compare
d3dbcbc
to
c0fd8ed
Compare
Contribution description
To better integrate RIOT into some automated tests, it can be useful to run commands in a one-shot fashion.
This allows to use RIOT shell commands like Linux applications and integrate them into other tooling so e.g. Linux servers that interact with RIOT can be tested using a fixed RIOT binary.
Testing procedure
First create a TAP bridge with an (optional) uplink:
We can
ping
a host from RIOTWe can also access CoAP servers
Issues/PRs references